accounts/usbwallet: add support for Ledger Nano Gen5#33297
Conversation
gballet
left a comment
There was a problem hiding this comment.
Left a few comments. Basically, this should just be a single new line in the code.
We will also need to test it, as we don't have a ledger nano gen5 handy.
| } | ||
| } | ||
| infos, err := hid.Enumerate(hub.vendorID, 0) | ||
| infos, err := usbEnumerate(hub.vendorID, 0) |
There was a problem hiding this comment.
this change is only used for the test, and the test isn't very useful. Please revert
| var ledgerProductIDs = []uint16{ | ||
| // Original product IDs | ||
| 0x0000, /* Ledger Blue */ | ||
| 0x0001, /* Ledger Nano S */ | ||
| 0x0004, /* Ledger Nano X */ | ||
| 0x0005, /* Ledger Nano S Plus */ | ||
| 0x0006, /* Ledger Nano FTS */ | ||
| 0x0007, /* Ledger Flex */ | ||
| 0x0008, /* Ledger Nano Gen5 */ | ||
|
|
||
| 0x0000, /* WebUSB Ledger Blue */ | ||
| 0x1000, /* WebUSB Ledger Nano S */ | ||
| 0x4000, /* WebUSB Ledger Nano X */ | ||
| 0x5000, /* WebUSB Ledger Nano S Plus */ | ||
| 0x6000, /* WebUSB Ledger Nano FTS */ | ||
| 0x7000, /* WebUSB Ledger Flex */ | ||
| 0x8000, /* WebUSB Ledger Nano Gen5 */ | ||
| } | ||
|
|
There was a problem hiding this comment.
don't move that out there, we will lose track of which commit added which device. Simply add new entries in the old table.
| deviceUsagePage = 0xffa0 | ||
| // deviceInterface identifies Ledger devices by USB interface number (0) on Linux. | ||
| deviceInterface = 0 | ||
| ) |
There was a problem hiding this comment.
unlike the rest, this is ok to define constants for this
There was a problem hiding this comment.
Good that you thought about writing a test, but this is just testing that enumeration matches what we already know, not what the system is going to return. Given that this test needs to swap the enumeration function - thus gutting a good chunk of the logic being tested, I suggest removing it.
adds support for the 0x0008 / 0x8000 product ID (Ledger Apex | Nano Gen5).
adds support for the 0x0008 / 0x8000 product ID (Ledger Apex | Nano Gen5). Co-authored-by: mmsqe <tqd0800210105@gmail.com>
adds support for the 0x0008 / 0x8000 product ID (Ledger Apex | Nano Gen5).